Skip to content

Conversation

@stevenzeck
Copy link
Contributor

This PR refactors how the OPDS catalog UI is built. Instead of using non-Compose pure Kotlin and adding to views programmatically, it's entirely XML based like the rest of the app. This unfortunately means it has more lines of code, but CatalogFragment is significantly smaller. And with the DiffUtil of RecyclerView, it seems to be a little bit faster than before. There are still improvements that can be made, but can come back to it later. OPDS would be a great candidate for Jetpack Compose, depending how much of it can be done in chunks (especially with the NavGraph being all Kotlin).

  1. Renamed item_recycle_catalog_list to item_recycle_button and removed data binding in it. This offers better reusability of it, and data binding doesn't seem to be the future anymore, so will probably move away from it. I still want to use view binding though, as it doesn't require any changes to the XML and is type safe.
  2. Each type of list on the OPDS catalog screen is a different RecyclerView, with different adapters: Publication, Group, and Navigation. The Group RecyclerView also uses the Publication and Navigation components in it.
  3. The Group RecyclerView has a larger title for the group name and a proper looking forward arrow
  4. This may have been something I originally changed and was overruled, but I did rename CatalogDetailFragment to PublicationDetailFragment. I think this more accurately reflects what it is, since it's not really the detail screen of the entire catalog. So now we have CatalogFeedListFragment which has the buttons for the different catalogs, CatalogFragment which has the contents of a specific catalog, and PublicationDetailFragment which serves as the detail screen for a specific publication

Copy link
Member

@mickael-menu mickael-menu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the code is definitely easier to follow. There are areas of improvements but I don't think it's worth spending time on it before the reimplementation with Jetpack Compose, which will be a great fit for the OPDS views.

... and removed data binding in it. This offers better reusability of it, and data binding doesn't seem to be the future anymore, so will probably move away from it. I still want to use view binding though, as it doesn't require any changes to the XML and is type safe.

Agreed, I avoid data binding but still rely on view binding for layout-based views, until migrating to Jetpack Compose.

@mickael-menu mickael-menu changed the title Refactor how the OPDS catalog UI is built Refactor the Test App OPDS catalog UI Oct 6, 2021
@mickael-menu mickael-menu merged commit 104d6d1 into readium:develop Oct 6, 2021
@stevenzeck stevenzeck deleted the opds-refactor branch October 6, 2021 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants